Repository navigation
fix(sidecar): confirm an unjail from the jail state when the tx index is off - #603
Conversation
… is off The PLT-1392 harbor e2e unjailed a validator: the tx landed and the validator reads bonded, but the task failed with 'inclusion unverifiable: node transaction indexing is disabled'. An Unjail task always targets a validator, and validators often run with the tx index off, so the task reported failure on a successful unjail. When the node cannot look up the tx, the handler now polls the jail state for up to 30 seconds. A validator that reads not jailed was released, and the task completes; the result keeps the tx hash and the unverifiable inclusion status. A validator that still reads jailed keeps today's terminal unverifiable error. Refs: PLT-1392 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@seidroid review |
PR SummaryMedium Risk Overview When Comments on Reviewed by Cursor Bugbot for commit 3259628. Bugbot is set up for automated code reviews on this repo. Configure here. |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
When the node has no tx index and cannot look up an unjail tx, the Unjail handler now polls the validator's jail state for up to 30s and completes the task once the validator reads released. The logic is correct and both outcomes are tested; what remains is a documentation gap in the wire enum and a loose bound on the wait, neither of which blocks. Reconciliation: I kept codex's one finding on releasedAfter's deadline as a nit, because each RPC already has a 30s client timeout (tmRPCTimeout), so the overrun is bounded and does not last until the outer context expires. Go was not available here, so I could not run the tests.
Non-blocking
1 finding on the changed lines, as inline comments.
1 nit, not posted on the code
sidecar/tasks/unjail.go:156— (Raised by codex, scope corrected.)confirmWaitis only checked between reads, andreadJailgets the engine's cancel-only ctx. One read near the deadline can issue up to four RPCs, each limited only by the 30stmRPCTimeout, so the 30s window can stretch to a couple of minutes. Also, read errors are dropped without being logged, so a failed read and a validator that is still jailed both end in the same "inclusion unverifiable" error. Acontext.WithDeadlinearound the loop, plus logging the last read error, would fix both.
seidroid review · decision approve · session 1b27e6b8799049038b6607b5cf6d370a · turn resp_claude_00364e3f9fd5d45cdebbd81e1dc93d04 · item 6391cb0bd22c53958714281caa4f8eb7
Findings: 0 blocking | 1 non-blocking | 1 posted inline
…d-unverifiable case Review follow-ups on #603: - releasedAfter wraps the poll in a context deadline, so a slow read near the end cannot stretch the 30-second wait, and it logs the last read error when it gives up. - wire.InclusionUnverifiable and classifyGovResult now say that an Unjail task can complete with this status once the validator reads released. Refs: PLT-1392 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@seidroid review |
There was a problem hiding this comment.
Since the last review, the wire.InclusionUnverifiable doc and the classifyGovResult comment now say an Unjail can complete with this status, which resolves the open thread. releasedAfter now runs under a context.WithTimeout(confirmWait), so the deadline bounds every read, and it logs the last read error when it gives up, which settles the earlier nit too. Nothing new blocks. codex's reading found nothing, which matches mine; Go was not available here, so I could not run the tests.
seidroid review · decision approve · session 1b27e6b8799049038b6607b5cf6d370a · turn resp_claude_b47bbd07b57e0cd4869deaf2e0c3cf9d · item 20d8b2e4c0f058288324c3548dd047b5
Findings: 0 blocking | 0 non-blocking | 0 posted inline
… is off (#604) * fix(sidecar): confirm a GovVote from the gov module when the tx index is off GovVote targets validators, and most run with the tx index off. The sidecar could not look up the vote tx, so the task ended Failed with 'inclusion unverifiable' even when the vote landed. #603 fixed the same case for Unjail. When the tx is unverifiable, the handler now polls the gov vote query (proposal, voter) for up to 30 seconds. A recorded vote with the requested option as its one full-weight choice completes the task; the result keeps the tx hash and the unverifiable inclusion status. A different or missing vote keeps the terminal error. The wire docs and the GovVote kind comment name the new exception. Refs: PLT-1401 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(sidecar): do not trust a vote read from a node that is catching up Review on #604: a lagging node can still show an older vote with the requested option after a newer vote replaced it. readVoteState checks /status first and returns an error while the node catches up, so the confirmation keeps polling, as the Unjail jail-state read does. Refs: PLT-1401 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Summary
The PLT-1392 harbor e2e unjailed a validator. The
MsgUnjaillanded, and the validator readsBOND_STATUS_BONDED, not jailed, with its operator sequence 1 → 2. The task still failed:An
Unjailtask always targets a validator, and validators often run with the tx index off. So the task reported failure on a successful unjail.Change
Unverifiable), the handler polls the jail state for up to 30 seconds, once a second.inclusionStatus: unverifiable, so the record stays honest about what was observed.Verification
gofmt,go vet,golangci-lint --new-from-merge-base, andgo test ./....make manifests generateleaves no diff.Harbor e2e so far (on #602's sidecar)
stays jailed until …; sequence unchangedis not jailed; sequence unchanged🤖 Generated with Claude Code